It's cool to sanitize @kelvinkipruto if it's not a lot of work. Remember, we're not getting content from end users i.e. the web like reddit. We're rendering content from a CMS managed and reviewed by trusted content editors.

@codex @claude Review

Claude finished @kelvinkipruto's task in 2m 1s —— View job

PR Review: feat(security): add XSS sanitization and security headers

• Gather context and understand the request
• Read changed files
• Analyze security implementation
• Review component changes
• Post comprehensive review feedback

Overall this is a solid, well-structured security PR. I have a few issues of varying severity to flag.

sanitizeEmbedHtml.js — Generally Good

The whitelist is thoughtfully constructed. A few notes:

iframe.src is allowed without URL protocol enforcement.
The xss library's built-in URL filtering applies to href but its behavior for src depends on whether the onTagAttr callback is customized. In xss@^1.0.15 with a custom whiteList, javascript: protocol in src is not automatically stripped. Add explicit URL validation for src:

onTagAttr(tag, name, value) {
  if (tag === "iframe" && name === "src") {
    try {
      const url = new URL(value);
      if (!["https:", "http:"].includes(url.protocol)) return ""; // strip
    } catch {
      return ""; // invalid URL
    }
  }
},

Fix this →

sandbox is not enforced on iframes.
The whitelist allows sandbox as an attribute but does not require it. An editor could paste an embed without a sandbox attribute, giving the iframe full browser permissions. Consider enforcing a default sandbox value via onTagAttr:

if (tag === "iframe" && name === "sandbox" && value === "") {
  return "allow-scripts allow-same-origin";
}

Or, if you want to ensure iframes are always sandboxed, add it in a post-processing step.

securityHeaders.mjs — Needs Attention

script-src 'unsafe-inline' https: largely defeats XSS protection.
This CSP allows inline scripts AND scripts from any HTTPS host. Combined with the XSS sanitization in this PR, there is a design tension: you're sanitizing HTML to prevent script injection but the CSP would allow inline scripts anyway. The comment notes MUI requires this, but this is worth revisiting — MUI v6 supports nonce-based CSP. This is a known limitation, but it should be documented explicitly and tracked as a follow-up item.

connect-src 'self' https: allows connections to any HTTPS URL in production. If the app doesn't need to call arbitrary third-party APIs from the browser, this should be scoped to specific domains (Payload CMS API, Sentry, etc.).

frame-ancestors 'self' and X-Frame-Options: SAMEORIGIN are redundant.
Not a bug — having both is correct for legacy browser compatibility. Just a note for awareness.

securityHeaders constant is evaluated at import time (line 67).
export const securityHeaders = getSecurityHeaders() captures process.env.NODE_ENV at module load. This is fine for a Next.js config but subtle. The exported getSecurityHeaders function is the safer API; the securityHeaders constant is a convenience that could surprise someone testing with a mocked NODE_ENV. Consider removing the pre-evaluated constant and only exporting the function, calling it in next.config.mjs.

Component Changes

HelplineEmbedDialog accepts unsanitized input.
The dialog (HelplineEmbedDialog.js) receives embedCode and renders it with dangerouslySetInnerHTML without sanitizing it. Both HelplineCard and RowCard currently pass a pre-sanitized value, but this creates a fragile contract — any future caller that passes raw CMS data directly to HelplineEmbedDialog will bypass sanitization silently.

Move the sanitization into HelplineEmbedDialog itself (or document clearly in a comment that callers must sanitize). Fix this →

No memoization for sanitization.
sanitizeEmbedHtml is called on every render in all three components. For typical embed strings this is negligible, but wrapping it in useMemo is cleaner and communicates that it's an expensive/intentional transformation:

const sanitizedEmbedCode = useMemo(() => sanitizeEmbedHtml(embedCode), [embedCode]);

payload.config.ts — Clean Improvement ✓

The refactor to normalizeOrigins, trailing-slash stripping, and the fallback to [defaultAllowedOrigin] when env vars are absent are all good changes. The serverURL fix (replace(/\/+$/, "") vs slice(0, -1)) is more robust.

One observation: normalizeOrigins deduplicates repeated slashes (replace(/\/+$/…)) but does not deduplicate repeated entries in the array. Not a bug, just worth noting.

On kilemensi's comment

The reviewer's point is valid — trusted CMS content is lower risk than user-generated content. That said, "defense in depth" is a legitimate reason to sanitize even trusted content (editor accounts can be compromised, CMS storage can be modified, etc.). The sanitization overhead is small and the protection is real.

What weakens the argument is the current CSP ('unsafe-inline' https:) — if you fix that, the sanitization becomes much more meaningful. As-is, the CSP gap limits the XSS protection provided by this PR.

Summary

Remember, we're not getting content from end users i.e. the web like reddit. We're rendering content from a CMS managed and reviewed by trusted content editors.

My question still remains @kelvinkipruto , why go through all of this if the content comes from our CMS which we have full control of? The content doesn't come from the wild web out there.